feat(v2): publish the IAP sign-in page from the deployed image - #433
Draft
mattdrees wants to merge 4 commits into
Draft
feat(v2): publish the IAP sign-in page from the deployed image#433mattdrees wants to merge 4 commits into
mattdrees wants to merge 4 commits into
Conversation
Enough of the Docker Registry v2 API to support publishing an artifact that ships inside the app image. Reads the config blob for labels, then scans layers newest-first for one known path, stopping at the first hit -- so a file COPYed late in a Dockerfile costs one small blob fetch rather than a full image pull. No docker daemon, no crane binary. src/v2/tar.js is a minimal ustar reader rather than a dependency: the only caller wants one file at a fixed, short, conventional path, and a shared actions repo pays for every transitive dep in review surface. PAX and GNU long-name records are skipped, not interpreted -- documented, and harmless for a 22-character path. authClient is exported from src/v2/gcp.js so the registry reads reuse the same ADC credentials and scope as the Artifact Registry REST calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With iap.friendly_signin, Terraform owns the sign-in bucket and the LB routing but something has to own the bytes. That was a workflow in the app repo, which does not survive v2: prod promotes an already-built image, so no app-repo workflow fires on a prod deploy (the upload was a manual dispatch), and a rollback re-deployed an old image behind a page built for a newer one. So the page travels in the image and the deploy publishes it -- same digest, same guarantees as the app. promote and rollback carry the matching page for free, and the only identity that can write to this world-readable, pre-auth bucket is the deployer rather than any app-repo workflow dispatch. Contract: LABEL org.cru.iap-signin=<object key> plus the built page at /cru/iap-signin/<object key>. The label is the single source of truth for the key, so image and bucket cannot disagree about the object name. Detection is bucket-first: Terraform already injects IAP_SIGNIN_BUCKET into the app container, so its presence on the pre-update service spec is the signal. No new action input, and no registry read for the apps that have no page. Never fatal. The page is static, cosmetic and pre-auth -- the module README calls a few minutes of staleness a non-outage -- so failing a production promote over it would trade a real problem for a trivial one. Publishes last so a partly-failed deploy cannot leave a new page in front of an old app, and warns on anything that goes wrong. A missing label warns rather than errors: that is the expected state when rolling back to a release built before this contract existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Matt: this is an idea I want to chat about with Brian and team sometime. Doesn't need immediate feedback.
Claude:
Summary
Makes the
deployaction publish the IAP friendly sign-in page out of the imageit is deploying, for apps that have one (
iap.friendly_signinon thegcp/cloudrun/appmodule).Today the app repo owns that upload. That does not survive v2:
app-repo workflow fires on a prod deploy. Today's upload is a manual dispatch —
bills' workflow says so in its header comment.
CSS, so it is version-specific, but nothing tied it to the deployed digest.
So the page travels inside the image and the deploy publishes it. Same
artifact, same digest, same guarantees as the app: promote and rollback carry the
matching page for free.
Secondary benefit worth naming: this bucket is the one world-readable,
unauthenticated surface of an otherwise-gated app, sitting on the app's origin
directly in front of the sign-in flow. The module README already treats "a bucket
the app's CI can write" as a phishing surface. This narrows the writers from any
app-repo workflow dispatch to the deploy path only.
Contract with the app image
The label value is the GCS object key and is the single source of truth for it,
so image and bucket cannot disagree about the object name.
Detection is bucket-first. Terraform already injects
IAP_SIGNIN_BUCKETintothe app container, so the deploy reads it off the pre-update service spec. No new
action input, no app-info field, and no registry read at all for the apps
that have no sign-in page.
Extraction reads one blob, not the image.
src/v2/oci.jsspeaks just enoughof the Docker Registry v2 API to read the config blob (labels) and then scan
layers newest-first for the file, stopping at the first hit.
src/v2/tar.jsis aminimal ustar reader rather than a new dependency — one fixed short path is all
it has to handle.
Verified against a real bills image built with this change: found on the first
of 19 layers, 87KB fetched vs a 346MB image, byte-identical to the built page.
Failure is never fatal. The page is static, cosmetic and pre-auth — the module
README calls a few minutes of staleness a non-outage — so failing a production
promote over it would trade a real problem for a trivial one. The publish runs
last (a deploy that fails partway must not leave a new page in front of an old
app) and anything that goes wrong becomes a
::warning::annotation. A missinglabel warns rather than errors: that is the expected state when rolling back to a
release built before this contract existed.
Companion PRs
cru-terraform-modules#698(the IAP module PR, already draft) — grants theper-env
cru-deploySAobjectAdminon the sign-in bucket. Merge beforethis lands anywhere real, or the publish step warns on every deploy.
The v1 grant and the
IAP_SIGNIN_BUCKETActions variable stay in the module fornow; beacon still uploads its own page on v1. Both come out when beacon migrates.
Decisions I made that are worth a second opinion
annotation. The alternative — failing the deploy — makes prod skew impossible
but lets a cosmetic page block a promote. I went with warn; say the word and
it becomes fatal.
deployCloudRunreturns the publish result but thedeployaction does not expose it as an output, so a skipped publish shows uponly as a run annotation, not in the promote/rollback Slack message. Noted as
a follow-up in
docs/pipeline-v2.mdrather than expanding this diff acrossthree workflows.
tar-streamdependency ina shared actions repo. It skips PAX/GNU long-name records rather than
interpreting them, which is safe for a fixed 22-character path and documented
as such. Happy to swap in the dep if you'd rather.
aws/cloudfront/sso-static-site, so there isno ECS/Lambda equivalent to add.
Note on the red CI check
Lint, Build & Testfails atnpm ciwith a pre-existing lockfile drift onpipeline-v2, unrelated to this branch:Every recent
pipeline-v2commit'sBuild dist/run fails the same way. I leftpackage-lock.jsonalone rather than fold an unrelated dependency fix into thisdiff — say if you'd rather I regenerate it here.
Locally, against the same tree with deps installed via
npm install --no-save:371 tests pass (306 before, +65 here),
npm run lintclean,npm run buildregenerates
dist/.